Skip to content

Improve documentation#9

Open
reachfh wants to merge 2 commits intoklarna:masterfrom
cogini:documentation
Open

Improve documentation#9
reachfh wants to merge 2 commits intoklarna:masterfrom
cogini:documentation

Conversation

@reachfh
Copy link
Copy Markdown

@reachfh reachfh commented Oct 26, 2019

Improve documentation / comments. No code changes.

download(Ref) ->
gen_server:call(?SERVER, {download, Ref}, infinity).

%% Download schema from registry and insert into cache.
Copy link
Copy Markdown

@k32 k32 Oct 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing @doc or @private tag?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added @doc tag, though it's a private function.

@k32
Copy link
Copy Markdown

k32 commented Oct 28, 2019

Hi, thanks for the PR! And apologies for slow responses. I have only one small comment, everything looks good otherwise.

%% bytes overhead), but relies on the registry.
%%
%% The fingerprint is more static, and can be obtained at compile
%% time, but it is harder to share with other applications and evolve
Copy link
Copy Markdown
Contributor

@zmstone zmstone Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To share schema by fp, it is not harder than registration ID. Since they are essentially just two numbers.
One may argue that without fp as a part of the encoded payload makes it harder to share (maybe that’s what you meant)
But I would phrase it as more freedom on choosing means to share.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g fp as payload meta data (it’s common to have envelops )
Or fp in kafka message header. Etc.
Even we in some cases did double registration: the id returned from fp registration can also be used to share the schema.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants